Skip to content

Conversation

@Rahul2322-P
Copy link

Fixed the bug where Ctrl+C was copying the file instead of the selected text in the Details Pane.
Changed args.Handled = true to args.Handled = false in OnCopyInvoked().
Now, when a text field is focused, Ctrl+C copies the text as expected.

if (!App.AppModel.IsMainWindowClosed && sender is WindowEx window)
{
args.Handled = true;
args.Handled = false;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Setting args.Handled = false in PropertiesWindow_Closed() allows window destruction, reintroducing known access violations.
Severity: CRITICAL | Confidence: 1.00

🔍 Detailed Analysis

The PropertiesWindow_Closed() event handler is modified to set args.Handled = false. This change allows the properties window object to be destroyed upon closure. This directly reverts a previous workaround implemented to prevent access violations, which were observed when the window was destroyed, particularly for inactive or background windows. The original workaround involved setting args.Handled = true to prevent destruction and instead cache the window, addressing issue #12057.

💡 Suggested Fix

Revert args.Handled = false to args.Handled = true in the PropertiesWindow_Closed() event handler to prevent window destruction and maintain the existing workaround for issue #12057.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/Files.App/Utils/Storage/Helpers/FilePropertiesHelpers.cs#L156

Potential issue: The `PropertiesWindow_Closed()` event handler is modified to set
`args.Handled = false`. This change allows the properties window object to be destroyed
upon closure. This directly reverts a previous workaround implemented to prevent access
violations, which were observed when the window was destroyed, particularly for inactive
or background windows. The original workaround involved setting `args.Handled = true` to
prevent destruction and instead cache the window, addressing issue #12057.

Did we get this right? 👍 / 👎 to inform future reviews.

@yaira2
Copy link
Member

yaira2 commented Nov 4, 2025

@Rahul2322-P thank you for the PR. I'm not seeing how this solves the issue, can you please clarify?

@yaira2 yaira2 added the changes requested Changes are needed for this pull request label Nov 4, 2025
@yaira2
Copy link
Member

yaira2 commented Nov 4, 2025

This PR is superseded by #17825

@yaira2 yaira2 closed this Nov 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changes requested Changes are needed for this pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants